-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
filemanager: annotate portal run id #529
Conversation
…l-run-id # Conflicts: # config/config.ts # lib/workload/stateless/statelessStackCollectionClass.ts # package.json # yarn.lock
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
} | ||
|
||
req = req.WithMethod("PATCH").WithS3Endpoint().WithQuery(url.Values{ | ||
"key": {fmt.Sprintf("*%v*", event.Detail.PortalRunId)}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could add the /
before & after the portalRunId to make sure we are hitting the intended path part and not some random filename / string in some other part of the key.
But I guess the chances of that happening are negligible, especially since we are not using the portalRunId for anything else in the key....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea.
As an additional comment, at some point it might be nice for the WorkflowRunStateChange
event to also contain an outputPath
where objects are expected to turn up. This would allow this service to query the filemanager API with /api/v1/s3?key=<output_path>/<portal_run_id>/*
, which should be faster because it will hit the index that supports prefix matching, and it would probably be even more robust.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm,...
To some extend that's already in place... WorkflowRunStateChange events can contain a Payload
JSON which may contain that information. However, this is not enforced nor standardised. That's on purpose (at least for now) to keep the use cases flexible. We may change that in the future...
Also, I am not sure that would always work... In the normal case the portalRunId update would happen immediately after the objects have been written so we would not expect a key change. However, especially the prefix of the key can change (e.g. hot storage vs archive) and we try to focus on the key part starting from the portalRunId.
So depending on when this procedure is run, this may or may not work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay makes sense. I think we leave it for now and see if it emerges later as a possibility.
ec53d74
to
1d2baf1
Compare
1d2baf1
to
f14bcc2
Compare
Recent updates
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM other than a bit too repetitive error control ;)
Closes #528
Changes
WorkflowRunStateChange
events, forSUCCEEDED
,FAILED
, andABORTED
states, and calls the filemanager API with aPATCH
request for updating the attributes using a/api/v1/s3?key=*<portal_run_id>*
query.GoFunction
, with the filemanager endpoint, and reference to the secret containing the JWT token.compose.yml
as it was not possible to launch the original compose file due to the build requiring access to a postgres database. This whole setup would be simplified if filemanager didn't need access to a development database to compile code.